From: Colin Walters Date: Mon, 5 Dec 2016 22:22:46 +0000 (-0500) Subject: lib: Always checksum content in deltas X-Git-Tag: archive/raspbian/2022.1-3+rpi1~1^2~4^2~42^2~21 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/%22/%22http:/www.example.com/cgi/%22?a=commitdiff_plain;h=4c8fc92aa02dbbd474009b61ca54cf49ef29da56;p=ostree.git lib: Always checksum content in deltas This is a follow up to conversation on list - in practice, if we're backing away from summary signing, then it makes sense to remove the special casing for checksums in deltas around summary signatures. This is also related to the recent change to enable GPG checking for commits in deltas - now we have a more coherent story between the previous pull path and deltas. I didn't do any performance checking, and while it's slightly annoying that we're now doing sha256 on the delta content twice (once for the part and once per object)...sha256 is pretty fast, I think most users are I/O bound anyways, and it'd drop even farther if we started using openssl. Closes: #612 Approved by: jlebon --- diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index ffa387a9..80c70438 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1010,8 +1010,6 @@ static_deltapart_fetch_on_complete (GObject *object, _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, part, - /* Trust checksums if summary was gpg signed */ - pull_data->gpg_verify_summary && pull_data->summary_data_sig, pull_data->cancellable, on_static_delta_written, fetch_data); @@ -1661,7 +1659,6 @@ process_one_static_delta (OtPullData *pull_data, g_autoptr(GBytes) inline_part_bytes = NULL; guint64 size, usize; guint32 version; - const gboolean trusted = pull_data->gpg_verify_summary && pull_data->summary_data_sig; header = g_variant_get_child_value (headers, i); g_variant_get (header, "(u@aytt@ay)", &version, &csum_v, &size, &usize, &objects); @@ -1731,7 +1728,6 @@ process_one_static_delta (OtPullData *pull_data, _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, inline_delta_part, - trusted, pull_data->cancellable, on_static_delta_written, fetch_data); diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 133ab016..5ef4df90 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -435,8 +435,7 @@ ostree_repo_static_delta_execute_offline (OstreeRepo *self, } if (!_ostree_static_delta_part_execute (self, objects, part, skip_validation, - FALSE, NULL, - cancellable, error)) + NULL, cancellable, error)) { g_prefix_error (error, "Executing delta part %i: ", i); goto out; @@ -653,7 +652,7 @@ show_one_part (OstreeRepo *self, (guint64)g_variant_n_children (ops)); if (!_ostree_static_delta_part_execute (self, objects, - part, TRUE, TRUE, + part, TRUE, &stats, cancellable, error)) goto out; diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h index 31e8971e..6121c482 100644 --- a/src/libostree/ostree-repo-static-delta-private.h +++ b/src/libostree/ostree-repo-static-delta-private.h @@ -141,7 +141,6 @@ typedef struct { gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, GVariant *header, GVariant *part_payload, - gboolean trusted, gboolean stats_only, OstreeDeltaExecuteStats *stats, GCancellable *cancellable, @@ -150,7 +149,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, void _ostree_static_delta_part_execute_async (OstreeRepo *repo, GVariant *header, GVariant *part_payload, - gboolean trusted, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index ff5a8a1a..a71e070f 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -39,7 +39,6 @@ G_STATIC_ASSERT (sizeof (guint) >= sizeof (guint32)); typedef struct { - gboolean trusted; gboolean stats_only; OstreeRepo *repo; guint checksum_index; @@ -180,7 +179,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, GVariant *objects, GVariant *part, - gboolean trusted, gboolean stats_only, OstreeDeltaExecuteStats *stats, GCancellable *cancellable, @@ -200,7 +198,6 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, state->repo = repo; state->async_error = error; - state->trusted = trusted; state->stats_only = stats_only; if (!_ostree_static_delta_parse_checksum_array (objects, @@ -296,7 +293,6 @@ typedef struct { GVariant *part; GCancellable *cancellable; GSimpleAsyncResult *result; - gboolean trusted; } StaticDeltaPartExecuteAsyncData; static void @@ -323,7 +319,6 @@ static_delta_part_execute_thread (GSimpleAsyncResult *res, if (!_ostree_static_delta_part_execute (data->repo, data->header, data->part, - data->trusted, FALSE, NULL, cancellable, &error)) g_simple_async_result_take_error (res, error); @@ -333,7 +328,6 @@ void _ostree_static_delta_part_execute_async (OstreeRepo *repo, GVariant *header, GVariant *part, - gboolean trusted, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) @@ -344,7 +338,6 @@ _ostree_static_delta_part_execute_async (OstreeRepo *repo, asyncdata->repo = g_object_ref (repo); asyncdata->header = g_variant_ref (header); asyncdata->part = g_variant_ref (part); - asyncdata->trusted = trusted; asyncdata->cancellable = cancellable ? g_object_ref (cancellable) : NULL; asyncdata->result = g_simple_async_result_new ((GObject*) repo, @@ -517,9 +510,9 @@ dispatch_bspatch (OstreeRepo *repo, return ret; } -/* When processing untrusted static deltas, we need to checksum the - * file content, which includes a header. Compare with what - * ostree_checksum_file_from_input() is doing too. +/* Before, we had a distinction between "trusted" and "untrusted" deltas + * which we've decided wasn't a good idea. Now, we always checksum the content. + * Compare with what ostree_checksum_file_from_input() is doing too. */ static gboolean handle_untrusted_content_checksum (OstreeRepo *repo, @@ -530,13 +523,10 @@ handle_untrusted_content_checksum (OstreeRepo *repo, g_autoptr(GVariant) header = NULL; g_autoptr(GFileInfo) finfo = NULL; gsize bytes_written; - - if (state->trusted) - return TRUE; finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid); header = _ostree_file_header_new (finfo, state->xattrs); - + state->content_checksum = g_checksum_new (G_CHECKSUM_SHA256); if (!_ostree_write_variant_with_size (NULL, header, 0, &bytes_written, state->content_checksum, @@ -579,26 +569,16 @@ dispatch_open_splice_and_close (OstreeRepo *repo, metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype), state->payload_data + offset, length, TRUE, NULL, NULL); - if (state->trusted) - { - if (!ostree_repo_write_metadata_trusted (state->repo, state->output_objtype, - state->checksum, - metadata, - cancellable, - error)) - goto out; - } - else - { - g_autofree guchar *actual_csum = NULL; + { + g_autofree guchar *actual_csum = NULL; - if (!ostree_repo_write_metadata (state->repo, state->output_objtype, - state->checksum, - metadata, &actual_csum, - cancellable, - error)) - goto out; - } + if (!ostree_repo_write_metadata (state->repo, state->output_objtype, + state->checksum, + metadata, &actual_csum, + cancellable, + error)) + goto out; + } } else { @@ -672,29 +652,18 @@ dispatch_open_splice_and_close (OstreeRepo *repo, &object_input, &objlen, cancellable, error)) goto out; - - if (state->trusted) - { - if (!ostree_repo_write_content_trusted (state->repo, - state->checksum, - object_input, - objlen, - cancellable, - error)) - goto out; - } - else - { - g_autofree guchar *actual_csum = NULL; - if (!ostree_repo_write_content (state->repo, - state->checksum, - object_input, - objlen, - &actual_csum, - cancellable, - error)) - goto out; - } + + { + g_autofree guchar *actual_csum = NULL; + if (!ostree_repo_write_content (state->repo, + state->checksum, + object_input, + objlen, + &actual_csum, + cancellable, + error)) + goto out; + } } } @@ -920,8 +889,6 @@ dispatch_close (OstreeRepo *repo, { const char *actual_checksum = g_checksum_get_string (state->content_checksum); - g_assert (!state->trusted); - if (strcmp (actual_checksum, state->checksum) != 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,